fix(conflict-resolution): document default source_priority=100 footgun + SOURCE_PRIORITY_ESCALATION warning (#1838)#1853
Merged
markmhendrickson merged 1 commit intoJun 30, 2026
Conversation
…n + SOURCE_PRIORITY_ESCALATION warning (#1838) source_priority defaults to 100 (z.number().optional().default(100)). An unprioritized write silently inherits 100 and, under a highest_priority reducer, outranks any prior observation written with an explicit priority <= 99 -- a silent trust-escalation. Operator verdict: docs + a warning; do NOT change the (breaking) default. - docs: new "default source_priority = 100 footgun" subsection in docs/subsystems/conflict_resolution.md with mitigation (always pass an explicit --source-priority for priority-semantic writes) + #1838 ref. - CLI: --source-priority help on both store commands now states the default-100 escalation edge and points at the conflict-resolution docs. - warning: buildSourcePriorityEscalationWarning() in src/services/source_priority_warning.ts emits a non-blocking SOURCE_PRIORITY_ESCALATION store_warning when priority===100 AND a written field uses highest_priority. Wired into both store paths (src/actions.ts HTTP/API, src/server.ts MCP) right after the existing IGNORED warning. Limitation: zod applies .default(100) before the handler, so an explicit 100 is indistinguishable from an omitted value at write time; the advisory fires on both (framed as "default/non-explicit 100"). Tests: unit coverage for priorityHonouringFields / sourcePriorityMayEscalate / buildSourcePriorityEscalationWarning; integration assertions that the escalation warning fires on a default-100 write to a highest_priority field on both MCP and HTTP paths (52 tests green). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Docs previewPreview URL: https://dev.neotoma.io/pr-1853/ Built from |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The footgun
source_prioritydefaults to 100 (z.number().optional().default(100)insrc/shared/action_schemas.ts). On any field whose reducer policy ishighest_priority(ormost_specificwithtie_breaker: "source_priority"), an unprioritized write silently inherits priority 100 — which outranks any prior observation written with an explicit priority ≤ 99 (e.g. a trusted import at 50). The unprioritized write wins the field. This is a silent trust-escalation: a caller who never thought about priority can override a value that was deliberately ranked lower.Per the operator verdict on #1838, the default value is unchanged (lowering it would silently flip the winner of existing reductions everywhere — a breaking semantic change). The fix is detection + docs, not a new default.
The fix (docs + warn only — no merge semantics change)
source_priority = 100footgun" subsection indocs/subsystems/conflict_resolution.md(§5 Current Limitations) with a worked example and the mitigation: always pass an explicit--source-priority/source_priorityfor priority-semantic writes. Added a Related-Documents entry for default source_priority is 100 — undocumented, causes silent trust-escalation for unprioritized writes #1838.--source-priorityhelp on bothstorecommands (src/cli/index.ts) now states the default-100 escalation edge and referencesdocs/subsystems/conflict_resolution.md.buildSourcePriorityEscalationWarning(...)insrc/services/source_priority_warning.tsreturns a{ code: "SOURCE_PRIORITY_ESCALATION", message, ... }(ornull). Condition:sourcePriority === 100and at least one written field useshighest_priority. Wired into both store paths —src/actions.ts(HTTP/APIstoreStructuredForApi) andsrc/server.ts(MCPstore) — right after the existingbuildSourcePriorityIgnoredWarningcall, pushed tostore_warnings[]. Non-blocking; nothing is rejected.Limitation (default vs. explicit 100)
zod applies
.default(100)before the request handler runs, so an explicitsource_priority: 100is indistinguishable from an omitted value at write time. The advisory therefore fires on both, framed as "default/non-explicit 100". This is called out in the warning message, the docs subsection, and the code comments.Tests
tests/unit/source_priority_ignored_warning.test.ts):priorityHonouringFields,sourcePriorityMayEscalate, andbuildSourcePriorityEscalationWarning(fires on default-100 + highest_priority; null on non-default priority; null when no honouring field;most_specifictie-breaker case; message names the honouring field and documents the limitation).tests/integration/store_source_priority_ignored_warning.test.ts): assertsSOURCE_PRIORITY_ESCALATIONfires on a default-100 write to ahighest_priorityfield on both the MCP and HTTP/storepaths, and does not fire on a non-default-priority write.type-check,lint(0 errors),prettier --check,validate:test-catalog, andvalidate:doc-depsall clean. No generated-file drift (openapi.yaml untouched; the docs audit lists paths only, no content/hash dependency).Closes #1838
🤖 Generated with Claude Code